-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Grouped Kibana nav #53545
Grouped Kibana nav #53545
Conversation
1fb0a08
to
c187078
Compare
💔 Build FailedHistory
To update your PR or re-run it, just comment with: |
Hey @myasonik I think this project is still on for 7.6. Running through the branch I think we'll likely need to make the following changes:
Icons are still on the way as are some small accessibility fixes from @thompsongl |
@snide Do you want this to happen in Kibana or in EUI? I think we could do it in either place... If we want this to always to be the case then I'd err towards doing this in EUI but if it's something special for Kibana we can do it on this side. |
Agree with @thompsongl. Let's handle it there. |
Just as an update for anyone tracking. elastic/eui#2749 will fix the not closing problems. I checked to make sure that features controls change the logic of how this stuff appears as written in the spec issue. Along with the remaining items from #53545 (comment) I added the advanced setting control, which might just be poorly labeled and I can't find it. Icons are still coming and possibly would need to merge into EUI post feature freeze. I don't think the icons alone should block merging this PR though if product is still OK with the functionality. |
An update on the current state:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elastic/kibana-app-arch changes LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Infra changes LGTM 👍
@elastic/kibana-platform Would love a re-review on Just did a bit of a refactor to encapsulate pieces better. Mostly didn't touch logic. #1 goal was to make thinking about testing easier. All of the those changes are in 06a2663. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-review as of #53545 (comment)
Extracting the header components is great. Thank for the cleanup.
Just a remark about the folder structure
🚨 Merging first thing Tuesday morning unless someone finds a reason not to before then |
@elasticmachine merge upstream |
UI copy in welcome screen LGTM. |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
Summary
Closes #53313
Dev docs
Plugins should now define a category if they have a navigation item:
DEFAULT_APP_CATEGORIES
defined insrc/core/utils/default_app_categories.ts
.AppCategory
interface defined insrc/core/types/app_category.ts
.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers